Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-application] Add subpath exports #384

Merged
merged 47 commits into from
May 8, 2024
Merged

[terra-application] Add subpath exports #384

merged 47 commits into from
May 8, 2024

Conversation

sdadn
Copy link
Contributor

@sdadn sdadn commented May 6, 2024 •
edited
Loading

Summary

This PR adds subpath exports (https://nodejs.org/api/packages.html#exports) to terra-application to follow better JavaScript conventions and practices. Now, terra-application can be imported a series of submodules instead of utilizing on the internal lib path. It also removes the default ApplicationBase export as that is already provided through the application-base submodule.

Note: while this has breaking changes, it will be released in the next minor version as terra-application v2 has not been fully deployed to production yet.

before:

import TerraApplication from 'terra-application';
import * from 'terra-application/lib/browser-router-redirect';
import * from 'terra-application/lib/content';
import * from 'terra-application/lib/layouts';
import * from 'terra-application/lib/loader-components';
import * from 'terra-application/lib/mdx';
import * from 'terra-application/lib/menu-button';
import * from 'terra-application/lib/modals';
import * from 'terra-application/lib/pages';
import * from 'terra-application/lib/shared';
import * from 'terra-application/lib/site';
import * from 'terra-application/lib/utils/loaders';
import * from 'terra-application/lib/utils/event-emitter';
import * from 'terra-application/lib/webpack';

After:

import * from 'terra-application/browser-router-redirect';
import * from 'terra-application/content';
import * from 'terra-application/layouts';
import * from 'terra-application/loader-components';
import * from 'terra-application/mdx';
import * from 'terra-application/menu-button';
import * from 'terra-application/modals';
import * from 'terra-application/pages';
import * from 'terra-application/shared';
import * from 'terra-application/site';
import * from 'terra-application/utils';
import * from 'terra-application/webpack';

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

In addition to testing these changes locally, this was also tested by installing these changes locally in a consuming project and running successfully running the jest tests and dev-site.

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

Jest started supporting subpath exports in v28, so a companion PR to upgrade Jest was also opened:
cerner/terra-toolkit#863

Thank you for contributing to Terra.
@cerner/terra

@sdadn sdadn self-assigned this May 6, 2024
@sdadn sdadn changed the title [terra-application] Add named exports [terra-application] Add subpath exports May 6, 2024
@sdadn sdadn marked this pull request as ready for review May 7, 2024 22:14
sycombs
sycombs approved these changes May 8, 2024
View reviewed changes
packages/terra-application-docs/src/dev-site-extended-test/component/ThemedComponent.jsx
@@ -1,7 +1,9 @@
/* eslint-disable import/no-unresolved */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, which import is causing the need for this? None of them are standing out to me as different from the other imports in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's both the imports

import { ApplicationIntlContext } from 'terra-application/application-intl';
import { ThemeContext } from 'terra-application/theme';

It's weird because they are working in other packages and webpack doesn't complain. If I change the name to something nonexistent like terra-application/theme-test, webpack gives an error so the imports do seem to be working correctly. So I'm not sure why the linter is failing.

@sdadn sdadn merged commit 1f52c9d into main May 8, 2024
6 checks passed
@sdadn sdadn deleted the add-named-exports branch May 8, 2024 17:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sycombs sycombs sycombs approved these changes

@adoroshk adoroshk adoroshk approved these changes

Assignees

@sdadn sdadn

Labels
Major Version Bump 📦 terra-application
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sdadn @sycombs @adoroshk